Skip to content

Rename Post to Grants#3947

Merged
lmazuel merged 2 commits intoAzure:masterfrom
lmazuel:graphrbac_rename
Sep 24, 2018
Merged

Rename Post to Grants#3947
lmazuel merged 2 commits intoAzure:masterfrom
lmazuel:graphrbac_rename

Conversation

@lmazuel
Copy link
Member

@lmazuel lmazuel commented Sep 19, 2018

@shanepeckham Our naming convention should not use HTTP verbs as part of operation ID, but be more meaningfull. This PR renames "post" to "grants", this will make SDK looks like:

client.oauth2_permissions.grants()

which is very better user friendly. I do believe your CLI PR is not merged yet, so I think it's the right time to do that now. I discussed this with your initial reviewer @marstr and he agreed on that.

@yugangw-msft FYI

@AutorestCI
Copy link

AutorestCI commented Sep 19, 2018 •
edited
Loading

Automation for azure-sdk-for-js

Nothing to generate for azure-sdk-for-js

@AutorestCI
Copy link

AutorestCI commented Sep 19, 2018 •
edited
Loading

Automation for azure-sdk-for-python

A PR has been created for you based on this PR content.

Once this PR will be merged, content will be added to your service PR:
Azure/azure-sdk-for-python#2032

@AutorestCI
Copy link

AutorestCI commented Sep 19, 2018 •
edited
Loading

Automation for azure-sdk-for-ruby

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-ruby#1584

@AutorestCI
Copy link

AutorestCI commented Sep 19, 2018 •
edited
Loading

Automation for azure-sdk-for-node

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-node#3682

@AutorestCI AutorestCI mentioned this pull request Sep 19, 2018
Merged
@AutorestCI
Copy link

AutorestCI commented Sep 19, 2018 •
edited
Loading

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

@AutorestCI
Copy link

AutorestCI commented Sep 19, 2018 •
edited
Loading

Automation for azure-sdk-for-go

Nothing to generate for azure-sdk-for-go

@azuresdkci
Copy link
Contributor

azuresdkci commented Sep 19, 2018

Can one of the admins verify this patch?

@lmazuel lmazuel assigned lmazuel and unassigned sarangan12 Sep 19, 2018
@lmazuel lmazuel requested review from marstr and removed request for sarangan12 September 19, 2018 16:29
marstr
marstr reviewed Sep 19, 2018
View reviewed changes
specification/graphrbac/data-plane/stable/1.6/graphrbac.json Outdated
"post" : {
"tags": [
"OAuth2Permissions_post"
"OAuth2Permissions_Grants"
Copy link
Member

@marstr marstr Sep 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making this plural suggests a read operation to me. I'd recommend "OAuth2Permissions_Grant" or even "OAuth2Permissions_CreateOrUpdate"

Copy link
Member Author

@lmazuel lmazuel Sep 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a POST action I would avoid "create_or_update", I go for "grant" :)

@AutorestCI AutorestCI mentioned this pull request Sep 19, 2018
Merged
@lmazuel lmazuel merged commit d62e149 into Azure:master Sep 24, 2018
@lmazuel lmazuel deleted the graphrbac_rename branch September 24, 2018 16:59
@lmazuel
Copy link
Member Author

lmazuel commented Oct 10, 2018

@AutorestCI rebuild azure-sdk-for-python

@AutorestCI AutorestCI mentioned this pull request Oct 10, 2018
Merged
lmazuel added a commit that referenced this pull request Oct 10, 2018
@lmazuel
Fix operation named missed in #3947 (#4160)
bf5d6f0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@marstr marstr marstr left review comments

@shanepeckham shanepeckham Awaiting requested review from shanepeckham

Assignees

@lmazuel lmazuel

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@lmazuel @AutorestCI @azuresdkci @marstr @sarangan12